-
Notifications
You must be signed in to change notification settings - Fork 161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restricted Cofunction RHS #3922
base: master
Are you sure you want to change the base?
Conversation
|
|
firedrake/variational_solver.py
Outdated
for c in F.coefficients(): | ||
if c.function_space() == V.dual(): | ||
replace_F[c] = Cofunction(V_res.dual()).interpolate(c) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand why you do this, but I would like to think a bit more to see if there is any better solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hoped something like self.F = interpolate(F, V_res.dual())
would work, but it looks not supported, so maybe this is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found a way to make self.F = action(replace(F, {u: u_res}), interpolate(v_res, V))
work. It requires fixing some FormSum bugs, and fixing the way assemble
optimizes and applies BCs on BaseForms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
…rakeproject/firedrake into pbrubeck/fix/restricted-cofunction
628fc95
to
0d70a66
Compare
0d70a66
to
40cf6d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix CI.
firedrake/variational_solver.py
Outdated
for c in F.coefficients(): | ||
if c.function_space() == V.dual(): | ||
replace_F[c] = Cofunction(V_res.dual()).interpolate(c) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hoped something like self.F = interpolate(F, V_res.dual())
would work, but it looks not supported, so maybe this is fine.
firedrake/cofunction.py
Outdated
@@ -225,8 +225,12 @@ def assign(self, expr, subset=None, expr_from_assemble=False): | |||
return self.assign( | |||
assembled_expr, subset=subset, | |||
expr_from_assemble=True) | |||
|
|||
raise ValueError('Cannot assign %s' % expr) | |||
elif expr == 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do the same for ufl.classes.Zero
on line 199 so this should probably be combined. In general I am a bit concerned about this code because we do not tape Assigner
. @Ig-dolci knows more I believe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I copied this from Function.assign
, and just noticed that Function.assign
is decorated with @FunctionMixin._ad_annotate_assign
. This is required for bc.set(Cofunction, float)
if we want to set diagonal entries to 1.
8ee8028
to
34669b7
Compare
34669b7
to
fe30b48
Compare
0acaa6e
to
6fdaf9d
Compare
6fdaf9d
to
df04f4b
Compare
8ada771
to
73be82b
Compare
73be82b
to
a86f3f5
Compare
Description
This PR enables
solve(a == assemble(L), u, bcs, restrict={False|True})
without requiring users to set the boundary nodes to zero when they provide a Cofunction RHS. Here we also solved a few other issues:assemble(form, bcs, zero_bc_nodes=True)
is the new default, and for the False case we throw an error because we will try to apply primal inhomogenous data on a dual Cofunction.Depends on firedrakeproject/ufl#53
Fixes #3203, #3498, and #3130
Supersedes #3214, #3754